Skip to content

fix: redact access token in config command output - #787

Open
rchaves-nexar wants to merge 2 commits into
launchdarkly:mainfrom
rchaves-nexar:fix/redact-access-token-in-output
Open

fix: redact access token in config command output#787
rchaves-nexar wants to merge 2 commits into
launchdarkly:mainfrom
rchaves-nexar:fix/redact-access-token-in-output

Conversation

@rchaves-nexar

@rchaves-nexar rchaves-nexar commented Sep 1, 2026

Copy link
Copy Markdown

Requirements

  • I have added test coverage for new or changed functionality
  • I have followed the repository's pull request submission guidelines
  • I have validated my changes against all supported platform versions

Related issues

Fixes #786.

Describe the solution you've provided

The stored access token was rendered verbatim by config --list and by the --set/--unset validation error. Two commits, one per path.

1. config --list. Adds Config.Redacted(), which returns a copy with sensitive values replaced by [REDACTED], and marshals that for output instead of the raw config. The plaintext renderer, --output json, and the JSON default used when stdout is not a terminal all derive from the same json.Marshal in cmd/config/config.go, so the single substitution covers all three.

Two details worth flagging for review:

  • The receiver is a value, so the copy is free and nothing that is written back to disk is affected. Verified end to end: the config file is byte-identical after --list.
  • An unset token is left empty rather than set to [REDACTED], so omitempty still elides the key. Reporting an absent token as "present but hidden" would be a worse answer than omitting it.

I chose an explicit method over reflecting on a new struct tag, or over unmarshalling to a map and substituting there. The map route re-sorts the keys and changes output shape for everyone; the method preserves field order and matches the hand-written switch already used in Update.

2. Validation errors. --set and --unset reject an unknown key by echoing it back. The argument in that position is not always a key — transposing --set <key> <value> puts the value there, and for access-token that value is a secret:

$ ldcli config --set api-<token> access-token
{"message":"api-<token> is not a valid configuration option"}

Rather than trying to detect secrets — a heuristic on token prefixes will rot the moment a token format changes — this checks the shape of what is about to be echoed. Configuration keys are short lowercase-hyphenated words; anything else is not a key, so there is no reason to reflect it back. The typo case that the echo exists to serve is unaffected:

$ ldcli config --set projct foo
{"message":"projct is not a valid configuration option"}     # unchanged

$ ldcli config --set api-<token> access-token
{"message":"[REDACTED] is not a valid configuration option"}  # fixed

Describe alternatives you've considered

  • A --show-secrets flag. Deliberately omitted. It weakens the guarantee to "safe unless someone passes a flag", and a flag whose purpose is to print a secret tends to end up in scripts. No capability is lost without it: the config file is plain YAML at a documented path, so yq '.["access-token"]' "$XDG_CONFIG_HOME/ldcli/config.yml" still works for anyone who genuinely needs the value. Happy to add the flag if you would rather not take the behavior change unconditionally.
  • Partial masking (api-••••••••cd12). Lets a human identify which token is stored, but still puts live secret characters into scrollback, and ldcli whoami already names the token without revealing it.
  • A registry-driven sensitive-key set in cmd/cliflags. Attractive since AllFlagsHelp() is already the single authority for valid config keys, but promoting it from map[string]string to a struct changes a shape used by the public config --help listing, for no benefit at one sensitive key. Easy to move to later if a second one appears.

Additional context

Behavior change, so calling it out plainly: any caller parsing the token out of config --list output will now read [REDACTED]. I believe that is the point of the change rather than a regression, but it is your call whether it needs more than a patch release.

cmd/config/testdata/help.golden is unchanged — no flag is added and no description is edited.

Tests added in internal/config/config_test.go:

  • Redacted() — token replaced when set; left empty when unset (asserted through json.Marshal, so omitempty is actually exercised); non-sensitive fields untouched; receiver not mutated.
  • TestRedactedOutput — marshals the redacted config through output.CmdOutputSingular for both json and plaintext, asserting the token appears in neither. This is what pins sites 2 and 3 rather than just the struct.
  • TestErrorDoesNotEchoNonKeyArguments — transposed --set and non-key --unset redact; a key-shaped typo still echoes verbatim.

I skipped an end-to-end config --list test on purpose: it needs viper.SetConfigFile on global state that cmd/root.go owns, and a flaky test seemed worse than a missing one. I did verify the built binary by hand in all three output modes.

Verified locally: make build, go test ./..., go vet, and golangci-lint v1.63.4 all clean; gofmt -l reports nothing in the touched files. (cmd/root.go and cmd/dev_server/projects.go are flagged by gofmt on main already — I left them alone rather than mix unrelated formatting into this diff.)


Note

Overview
Stops the LaunchDarkly CLI from leaking the stored access token when users run config --list or hit invalid-key errors on config --set / --unset.

config --list now JSON-marshals Config.Redacted() instead of the live struct, so access-token appears as [REDACTED] in JSON and plaintext output while the on-disk YAML is unchanged. Unset tokens stay empty so omitempty still omits the field.

Validation errors no longer echo arbitrary rejected “keys”: safeKeyForError only repeats arguments that look like real config keys (short, lowercase-hyphenated); mistyped --set that puts the token in the key slot shows [REDACTED] instead of the secret.

Tests cover Redacted(), both output modes via CmdOutputSingular, and the error-path redaction behavior.

Reviewed by Cursor Bugbot for commit b737ff2. Bugbot is set up for automated code reviews on this repo. Configure here.

The access token stored in the config file was rendered verbatim by
`ldcli config --list` in every output mode, including the JSON default
used whenever stdout is not a terminal. That put the secret into terminal
scrollback, shell history captures, and piped CI logs.

Add Config.Redacted() and marshal that for output instead. Because the
plaintext, --output json and non-TTY JSON paths all derive from the same
marshal, one substitution covers all three. The value written to the
config file is unchanged, and an unset token stays elided by omitempty
rather than being reported as present but hidden.
`config --set` and `config --unset` reject an unknown key by echoing it
back. The argument in that position is not always a key: transposing
`--set <key> <value>` puts the value there, so `--set <token>
access-token` printed the token inside the error message.

Echo the argument only when it has the shape of a configuration key.
Typos, which are the reason the argument is echoed at all, still appear
verbatim.
@rchaves-nexar
rchaves-nexar requested a review from a team as a code owner September 1, 2026 09:52
@ffantl-ld
ffantl-ld self-requested a review September 1, 2026 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

config --list prints the stored access token in full

1 participant